Agent Health issue for OpenMetrics metrics dropped by configured limit - #24819
Conversation
|
nubtron
changed the title
The Fleet UI renders remediation step text as plain text, so the backticks around config option names were shown literally to customers. Match the plain-text convention used by other Agent Health producers.
The MetricLimitIssueReporter carried a boolean `legacy` flag whose only effect was selecting between the v1 (`metrics` / `ignore_metrics`) and v2 (`metrics` / `exclude_metrics`) config option names in remediation text. Pass the actual option string instead so the reporter no longer knows about OpenMetrics flavors and the call sites document themselves.
The Fleet UI renders Agent Health remediation text as plain text, so the
dotted form 'debug_metrics.metric_contexts: true' reads as a single literal
key. The check parses instance.get('debug_metrics', {}).get('metric_contexts'),
so a dotted top-level key would never match and no telemetry would be emitted.
Describe the nesting in prose ('set metric_contexts to true under the
debug_metrics section') and spell out both emitted metric names in full
instead of the orphaned '.limit'. Add assertions locking in the corrected
wording.
evalya-impact-summaryevalya impact analysis |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1584239ed8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…oints The V2 metric-limit callback read openmetrics_endpoint from the raw instance, which stays absent for integrations such as Cilium that synthesize scraper configs from agent_endpoint/operator_endpoint, so drops were never reported. Pass the actual configured scraper endpoint keys from self.scrapers instead, and have V1 pass its one effective prometheus_url. MetricLimitIssueReporter.handle now accepts a collection of endpoints and normalizes it to a deterministic, de-duplicated, sorted tuple, discarding empty values. One aggregate issue is reported for the whole run since the limiter state cannot attribute drops to a single scraper. The machine-readable extra['endpoints'] list and the issue identity reflect the endpoint set; the existing single-endpoint public issue id is preserved, and multi-endpoint identities hash the structured ordered collection.
|
Reviewed the change. It's well-built overall: limiter state is read before the 1. 2. Isolated checks are silently out of scope. Minor (non-blocking): |
The limiter's observed count covers the complete check run across all endpoints and telemetry contexts, so the remediation summary now addresses the check instance instead of a single endpoint.
Review from lavigne958 is dismissed. Related teams and files:
- container-integrations
- datadog_checks_base/datadog_checks/base/checks/openmetrics/metric_limit_issue.py
The isolated child process reconstructs the check from serialized configuration alone, so it must derive its over-limit behavior from config rather than parent-object state. IsolatedMetricLimitOpenMetricsCheck submits a fixed number of contexts, and test_isolated_check_reports_metric_limit_issue verifies the child reports the issue, the replay bridge forwards it to the parent Agent stub, and the parent's own limiter stays untouched.
Validation ReportAll 21 validations passed. Show details
|
|
@NouemanKHAL thanks for the review!
There are no other callers of issue_type in marketplace or extras, and we do want future callers to use
I'm not sure this is right , since the isolation branch calls the non-isolated branch in a new process. Added a test to verify that: d259c32
This is a good point, I changed the wording so that it says "instance" instead of "endpoint". |
|
Sounds good to me, I wasn't so sure about the 2nd one, figured to share the claude finding as you'd have more context. |
@NouemanKHAL it did expose a testing gap! Now we have have a proper process_isolation test! |
#24819) * Prototype OpenMetrics metric limit health issue * Add changelog for OpenMetrics health issue prototype * Refine OpenMetrics metric limit issue ownership * Scope OpenMetrics health issue IDs to host and check * Simplify OpenMetrics metric limit cleanup * Simplify metric limit hook ordering * Clarify OpenMetrics metric limit reporting * Simplify OpenMetrics metric limit remediation * Tighten OpenMetrics remediation wording * Resolve OpenMetrics health issues on clean runs * Report OpenMetrics health issues for any truncation * Route OpenMetrics health issues to Integrations * Remove Markdown backticks from remediation text The Fleet UI renders remediation step text as plain text, so the backticks around config option names were shown literally to customers. Match the plain-text convention used by other Agent Health producers. * Add IssueType to Agent Health reports * Align OpenMetrics IssueType with IssueName * Replace legacy flag with explicit metric filter config The MetricLimitIssueReporter carried a boolean `legacy` flag whose only effect was selecting between the v1 (`metrics` / `ignore_metrics`) and v2 (`metrics` / `exclude_metrics`) config option names in remediation text. Pass the actual option string instead so the reporter no longer knows about OpenMetrics flavors and the call sites document themselves. * Clarify debug_metrics.metric_contexts remediation as nested config The Fleet UI renders Agent Health remediation text as plain text, so the dotted form 'debug_metrics.metric_contexts: true' reads as a single literal key. The check parses instance.get('debug_metrics', {}).get('metric_contexts'), so a dotted top-level key would never match and no telemetry would be emitted. Describe the nesting in prose ('set metric_contexts to true under the debug_metrics section') and spell out both emitted metric names in full instead of the orphaned '.limit'. Add assertions locking in the corrected wording. * Clarify Agent Health issue name test * Remove redundant OpenMetrics reporter ownership test * Report OpenMetrics metric-limit drops against configured scraper endpoints The V2 metric-limit callback read openmetrics_endpoint from the raw instance, which stays absent for integrations such as Cilium that synthesize scraper configs from agent_endpoint/operator_endpoint, so drops were never reported. Pass the actual configured scraper endpoint keys from self.scrapers instead, and have V1 pass its one effective prometheus_url. MetricLimitIssueReporter.handle now accepts a collection of endpoints and normalizes it to a deterministic, de-duplicated, sorted tuple, discarding empty values. One aggregate issue is reported for the whole run since the limiter state cannot attribute drops to a single scraper. The machine-readable extra['endpoints'] list and the issue identity reflect the endpoint set; the existing single-endpoint public issue id is preserved, and multi-endpoint identities hash the structured ordered collection. * Skip metric limit handling in isolated parent * Simplify OpenMetrics multi-endpoint issue reporting * Clarify OpenMetrics filter option display text * Test remediation copy in its owning package * Clarify generic metric limit test fixture * Clarify OpenMetrics endpoint test setup * Clarify check-wide metric limit remediation The limiter's observed count covers the complete check run across all endpoints and telemetry contexts, so the remediation summary now addresses the check instance instead of a single endpoint. * Add end-to-end process isolation test for metric limit issue The isolated child process reconstructs the check from serialized configuration alone, so it must derive its over-limit behavior from config rather than parent-object state. IsolatedMetricLimitOpenMetricsCheck submits a fixed number of contexts, and test_isolated_check_reports_metric_limit_issue verifies the child reports the issue, the replay bridge forwards it to the parent Agent stub, and the parent's own limiter stays untouched. ded1457
3 participants
What does this PR do?
OpenMetrics stops submitting new metric contexts after it reaches
max_returned_metrics. The check logs a warning, but the resulting gaps can look intermittent and are easy to miss.This PR surfaces that condition in Agent Health. When an OpenMetrics v1 or v2 endpoint exceeds its effective limit, the check reports:
OpenMetrics Metrics Dropped By Configured Limitopenmetrics_metrics_dropped_by_configured_limitRemediation shown to customers
The issue carries the following remediation (from
metric_limit_issue._remediation):Summary: Reduce what this endpoint sends to Datadog, or raise this instance's metric limit after checking the cost.
metrics / ignore_metrics(OpenMetrics v1) ormetrics / exclude_metrics(OpenMetrics v2) on this instance to stop collecting series you do not query, alert on, or keep.max_returned_metricson this instance to a value above the observed count.debug_metrics.metric_contexts: trueon the instance to publishdatadog.agent.metrics.contexts.totaland.limit, and confirm the total stays below the limit at peak. Consider a monitor at 80% of the limit.Each endpoint gets a stable issue identity derived from the host, check name, endpoint, and namespace. The issue is refreshed while drops continue, resolved on the first clean collection, and reused if the condition later returns.
Severity reflects the fraction of contexts dropped:
The Agent Health bridge is best-effort: reporting failures do not fail the check or interfere with the limiter's existing cleanup. The generic
AgentCheckhook remains a no-op, so only OpenMetrics v1 and v2 opt into this issue.The PR also adds the missing
issue_typeargument toAgentCheck.report_issue. The Agent payload, persistence, and forwarding paths already support this field, so no rtloader or Agent ABI change is needed.Why?
When OpenMetrics exceeds
max_returned_metrics, some metrics are silently discarded after the warning is emitted. Because exporter ordering can change between scrapes, customers may see different metrics disappear over time instead of one obvious hard cutoff.Agent Health gives this failure mode a durable, endpoint-specific state with actionable remediation, while preserving the existing check lifecycle and limiter behavior.
Review checklist
qa/skip-qais applied